Sanitize all-numeric leading-zero short shas in generated versions - #247
Conversation
git-version emits <base>-<branch>.<n>.sha.<shortsha> on non-release branches. When the short sha is all-numeric with a leading zero (e.g. sha.0238954244), it is an invalid SemVer prerelease identifier — numeric identifiers must not have leading zeroes — and helm rejects the chart version, failing the pipeline (seen on codacy-website PR #2172). Prefix such shas with "g" (git-describe convention) so the identifier becomes alphanumeric, where leading zeroes are legal. Valid versions pass through untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a sanitization step to the version generation process, prefixing all-numeric short SHAs that have a leading zero with 'g' to ensure compliance with SemVer and prevent Helm chart rejection. The reviewer suggested extending this sanitization to the '.previous_version' file as well, which is generated by the same tool and could suffer from the same issue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # If the short sha happens to be all-numeric with a leading zero (e.g. sha.0238954244), | ||
| # it is an invalid SemVer prerelease identifier and helm rejects the chart version. | ||
| # Prefix such shas with "g" (git-describe convention) to keep the identifier alphanumeric. | ||
| command: sed -E -i 's/\.sha\.(0[0-9]+)$/.sha.g\1/' .version |
There was a problem hiding this comment.
The .previous_version file (generated on line 57) is also created by /bin/git-version and can suffer from the exact same invalid SemVer issue if its short sha is all-numeric with a leading zero.
To maintain consistency and prevent potential pipeline failures when the previous version is referenced, please add a similar sanitization step for .previous_version right after it is generated:
- run:
name: Sanitize previous version
command: sed -E -i 's/\.sha\.(0[0-9]+)$/.sha.g\1/' .previous_versionThere was a problem hiding this comment.
I do not think we use previous_version 🤔 for smth
so not doing it now
![codacy-production[bot]](https://avatars.githubusercontent.com/in/56611?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull Request Overview
The PR successfully implements a sanitization step using sed to ensure the current version string complies with SemVer rules by prefixing all-numeric, leading-zero short SHAs with 'g'. This prevents downstream tools like Helm from rejecting the version.
However, a significant gap exists: the sanitization logic is not applied to the .previous_version file generated later in the same job. This inconsistency means that any tool consuming the previous version (e.g., for rollbacks or comparisons) will still encounter validation failures if that version triggers the edge case. This should be addressed before merging to ensure holistic SemVer compliance.
About this PR
- There are no automated tests or verification steps included in the PR to confirm that the
sedregex behaves as expected across different SHA formats (e.g., all-numeric vs. alphanumeric). Consider adding a small verification step in the job to assert the format of the generated files.
1 comment outside of the diff
src/jobs/checkout_and_version.yml
line 57🟡 MEDIUM RISK
Apply the same sanitization logic to.previous_versionafter it is generated to ensure consistency and prevent SemVer validation failures in downstream logic (such as Helm charts or rollbacks). Currently, only the current version is fixed, leaving the previous version potentially invalid.Recommendation: Add a 'Sanitize previous version' step after the 'Set previous version' step using the same command:
sed -E -i 's/\.sha\.(0[0-9]+)$/.sha.g\1/' .previous_version.
Test suggestions
- A version ending in an all-numeric SHA with a leading zero (e.g., .sha.02389) is converted to .sha.g02389.
- A version ending in an all-numeric SHA without a leading zero (e.g., .sha.12389) remains unchanged.
- A version ending in an alphanumeric SHA starting with zero (e.g., .sha.023a9) remains unchanged.
- A standard release version (e.g., 1.0.0) remains unchanged.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. A version ending in an all-numeric SHA with a leading zero (e.g., .sha.02389) is converted to .sha.g02389.
2. A version ending in an all-numeric SHA without a leading zero (e.g., .sha.12389) remains unchanged.
3. A version ending in an alphanumeric SHA starting with zero (e.g., .sha.023a9) remains unchanged.
4. A standard release version (e.g., 1.0.0) remains unchanged.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
2 participants
Problem
checkout_and_versionwrites.versionfromgit-version, which emits<base>-<branch>.<n>.sha.<shortsha>on non-release branches. When a commit's short sha happens to be all-numeric with a leading zero (e.g.sha.0238954244), the version is invalid SemVer — numeric prerelease identifiers must not have leading zeroes — and helm rejects the chart version:Hit on codacy/codacy-website#2172. Rare (needs a 10-hex-digit sha of only digits starting with 0) but kills the pipeline when it lands, and the only workaround is pushing another commit to reroll the sha.
Fix
Add a
Sanitize versionstep right afterSet version: prefix such shas withg(git-describe convention), making the identifier alphanumeric — where leading zeroes are legal.Applied at the
.versionsource so docker tags, chart version, and version.sbt stay consistent. Valid versions (mixed alphanumeric shas, numeric shas without leading zero, release versions) pass through byte-identical.Alternative considered: fixing codacy/git-version to always emit
sha.g<short>— cleaner long-term, but changes every generated version string; this orb-side fix only touches the case that is broken today.🤖 Generated with Claude Code